Skip to content

Conversation

@alfetopito
Copy link
Collaborator

@alfetopito alfetopito commented Nov 13, 2025

Summary

Minor fix sorting chains in the same order as the UI.

To Test

  1. Open cow.fi on cowprotocol page and check the FAQ where all the chains are defined
  • Should list them in the same order as CoW Swap UI

Summary by CodeRabbit

  • Refactor
    • Improved consistency of chain display ordering.

@alfetopito alfetopito self-assigned this Nov 13, 2025
@vercel
Copy link

vercel bot commented Nov 13, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
cowfi Ready Ready Preview Nov 13, 2025 5:04pm
explorer-dev Ready Ready Preview Nov 13, 2025 5:04pm
sdk-tools Ready Ready Preview Nov 13, 2025 5:04pm
swap-dev Ready Ready Preview Nov 13, 2025 5:04pm
widget-configurator Ready Ready Preview Nov 13, 2025 5:04pm
1 Skipped Deployment
Project Deployment Preview Updated (UTC)
cosmos Ignored Ignored Nov 13, 2025 5:04pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 13, 2025

Walkthrough

Refactored the getAvailableChainsText function to iterate over SORTED_CHAIN_IDS and perform lookups in ALL_SUPPORTED_CHAINS_MAP instead of filtering ALL_SUPPORTED_CHAINS directly. The public function signature remains unchanged, but the underlying data access pattern and ordering mechanism are restructured.

Changes

Cohort / File(s) Summary
Chain Text Generation Refactor
libs/common-const/src/getAvailableChainsText.ts
Changed data source to use ALL_SUPPORTED_CHAINS_MAP with SORTED_CHAIN_IDS iteration; replaced direct filtering with reduce-based accumulation over sorted chain IDs; maintains same public API and output behavior

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Logic verification: Ensure the new reduce-based accumulation produces identical output to the previous filtering approach, particularly for edge cases (empty chains, single chain, testnet labeling).
  • Data consistency: Verify that ALL_SUPPORTED_CHAINS_MAP contains all chains referenced in SORTED_CHAIN_IDS to prevent runtime lookup failures.
  • Ordering impact: Confirm that sorting by SORTED_CHAIN_IDS preserves or improves the intended chain presentation order.

Possibly related PRs

Suggested labels

Partial approve

Suggested reviewers

  • elena-zh
  • cowdan
  • fairlighteth

Poem

🐰 Chains once scattered, now align,
Sorted IDs in perfect line,
Map lookups dance, reducing with grace,
Same output shines in reorganized space!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: sort chains according to the rest of the UI' directly and clearly summarizes the main change: ensuring chains are sorted consistently with the UI.
Description check ✅ Passed The description includes a summary of the change and testing instructions, though it lacks some template sections like background information and detailed checkbox-style verification steps.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/sort-chains

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bec1e54 and aa0ae58.

📒 Files selected for processing (1)
  • libs/common-const/src/getAvailableChainsText.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-05T14:27:05.023Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5992
File: libs/wallet/src/web3-react/utils/switchChain.ts:36-38
Timestamp: 2025-08-05T14:27:05.023Z
Learning: In libs/wallet/src/web3-react/utils/switchChain.ts, the team prefers using Record<SupportedChainId, string | null> over Partial<Record<SupportedChainId, string>> for WALLET_RPC_SUGGESTION to enforce that all supported chain IDs have explicit values set, even if some might be null. This ensures compile-time completeness checking.

Applied to files:

  • libs/common-const/src/getAvailableChainsText.ts
📚 Learning: 2025-09-19T11:38:59.206Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6232
File: apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx:199-200
Timestamp: 2025-09-19T11:38:59.206Z
Learning: The makeBuildClickEvent function in apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx takes five parameters: defaultChainId, contextLabel, mode, isSwapMode, and chainsCount. The chainsCount parameter is used to determine the CrossChain flag in analytics events.

Applied to files:

  • libs/common-const/src/getAvailableChainsText.ts
🧬 Code graph analysis (1)
libs/common-const/src/getAvailableChainsText.ts (1)
libs/common-const/src/chainInfo.ts (1)
  • SORTED_CHAIN_IDS (146-158)
🔇 Additional comments (2)
libs/common-const/src/getAvailableChainsText.ts (2)

1-3: LGTM! Import changes support the new ordering mechanism.

The switch to ALL_SUPPORTED_CHAINS_MAP enables efficient lookups, and SORTED_CHAIN_IDS provides the UI-consistent ordering needed to meet the PR objectives.


6-12: No changes needed. The code is type-safe.

ALL_SUPPORTED_CHAINS_MAP is typed as Record<SupportedChainId, ChainInfo>, which guarantees all SupportedChainId keys exist. Since SORTED_CHAIN_IDS contains only valid SupportedChainId enum members, the destructuring on line 7 is type-safe and cannot throw. The suggested diff is unnecessary.

Likely an incorrect or invalid review comment.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants